Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cni-server: do not perform ipv4 conflict detection during VM live migration #2693

Merged
merged 1 commit into from
Apr 24, 2023
Merged

cni-server: do not perform ipv4 conflict detection during VM live migration #2693

merged 1 commit into from
Apr 24, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented Apr 21, 2023 •
edited
Loading

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #2659

WHAT

🤖 Generated by Copilot at 02988f1

Improve pod migration and IP conflict detection in VLAN subnets. The change modifies handler.go to skip unnecessary ping checks and remove a redundant variable assignment.

🤖 Generated by Copilot at 02988f1

To move pods in VLANs without stress
We added detectIPConflict yes
We skip the ping check
To avoid a wreck
And removed a redundant excess

HOW

🤖 Generated by Copilot at 02988f1

  • Add a variable detectIPConflict to control IP conflict detection for pods in VLAN subnets (link)
  • Skip ping checks to the gateway for migrating pods in VLAN subnets (link, link)
  • Remove redundant assignment of detectIPConflict to true (link)

@zhangzujian zhangzujian added bug Something isn't working need backport labels Apr 21, 2023
@github-actions
Copy link
Contributor

  • The commit message is missing. It should provide a clear and concise description of the changes made in this patch.
  • There is a potential bug in the code. The detectIPConflict variable is being set to false unconditionally, which may cause IP conflicts to go undetected.
  • There is a format error in the code. The line detectIPConflict := podSubnet.Spec.Vlan != "" is repeated twice, once before the conditional statement and once inside it. This repetition should be removed.
  • There are no obvious performance issues in the code.
  • One way to improve the code would be to refactor the conditional statement that sets detectIPConflict to make it clearer and easier to understand.

@zbb88888 zbb88888 mentioned this pull request Apr 21, 2023
Closed
@github-actions
Copy link
Contributor

  • The commit message is missing. It should provide a clear and concise description of the changes made in this patch.
  • There is a potential bug in the code. The detectIPConflict variable is initialized to podSubnet.Spec.Vlan != "" before the if statement that checks for live migration. If the pod is being live migrated, the value of detectIPConflict will not be updated and may cause conflicts with other IP addresses.
  • There is a format error in the code. Line 271 has an extra space after the comma in the log message.
  • There is a performance issue in the code. The routes slice is being appended to itself, which can be slow for large slices. Consider using a separate slice to avoid this issue.
  • There is a way to improve the code. The if statement on line 241 can be simplified by combining the conditions with an && operator.

@zhangzujian zhangzujian marked this pull request as ready for review April 22, 2023 06:56
@zhangzujian zhangzujian merged commit f5fee4c into kubeovn:master Apr 24, 2023
@zhangzujian zhangzujian deleted the fix-vm-migration branch April 24, 2023 07:39
zhangzujian added a commit to zhangzujian/kube-ovn that referenced this pull request Apr 24, 2023
@zhangzujian
cni-server: do not perform ipv4 conflict detection during VM live mig…
22367be 
…ration (kubeovn#2693)
zhangzujian added a commit to zhangzujian/kube-ovn that referenced this pull request Apr 24, 2023
@zhangzujian
cni-server: do not perform ipv4 conflict detection during VM live mig…
d3311a7 
…ration (kubeovn#2693)
zhangzujian added a commit to zhangzujian/kube-ovn that referenced this pull request Apr 24, 2023
@zhangzujian
cni-server: do not perform ipv4 conflict detection during VM live mig…
42da9dd 
…ration (kubeovn#2693)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zbb88888 zbb88888 zbb88888 approved these changes

@oilbeater oilbeater Awaiting requested review from oilbeater

Assignees
No one assigned
Labels
bug Something isn't working need backport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

arp conflict check err
2 participants
@zhangzujian @zbb88888